Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

typescript(vx-shape): Re-write package in TypeScript #507

Merged
merged 22 commits into from
Oct 10, 2019

Conversation

williaster
Copy link
Collaborator

@williaster williaster commented Sep 20, 2019

🚀 Enhancements

This PR builds off #488 which introduces Typescript build config, and re-writes the @vx/shape package in TypeScript. Unfortunately this package is one of the biggest ones, so this PR is pretty sizable and larger than the other re-writes.

A couple of notes:

  • I added @types/d3-path and @types/d3-shape dependencies so that we can leverage/propagate existing types in our d3 usage.
  • I removed the @vx/point dependency .
  • I tried consolidating some of the duplicate code for the 12 link components, this was trickier than I thought so mostly share prop Types.

💥 Breaking Changes

  • This PR introduces React.Fragments, which requires bumping the peerDep for react to ^16.3.0-0
  • @vx/shape the Arc centroid prop was removed as it was not functional (it was called as if it was an arc.centroid() configuration parameter, but in reality the .centroid method is for returning the centroid of a datum.
  • @vx/shape the Area component is no longer wrapped in an empty <g> element
  • propTypes across all components are likely stricter.
  • children(...) render function overrides are wrapped in React.Fragments <>{children(...)</> to satisfy Types.
  • order and offset props for Stack, BarStack, BarStackHorizontal, and AreaStack previously supported strings, arrays, or functions. Only the string prop was functional, and only the enumerated string presets are now allowed.

Closes #506

Testing

  • CI
    • typescript
    • eslint
    • jest
  • Functional with /gallery
  • Generates .d.ts files

@hshoff @schillerk @milesj @techniq @kristw

value: number | NumAccessor,
) {
if (typeof value === 'number') func(value);
else func(value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is so weird. asked mohsen about it and he wasn't sure 🤔

packages/vx-shape/src/shapes/Area.tsx Outdated Show resolved Hide resolved
packages/vx-shape/src/shapes/link/line/LinkRadialLine.tsx Outdated Show resolved Hide resolved
packages/vx-shape/src/shapes/link/step/LinkRadialStep.tsx Outdated Show resolved Hide resolved
@hshoff hshoff added this to the v0.0.193 milestone Oct 9, 2019
@williaster
Copy link
Collaborator Author

whew. lots of test errors that didn't always show up locally 🤔

data,
path,
x = (d: $TSFIXME) => d.y, // note this returns a y value
y = (d: $TSFIXME) => d.x, // note this returns a y value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a blocker but the second commend should be x value

@hshoff hshoff merged commit 39074af into master Oct 10, 2019
@hshoff hshoff deleted the chris--typescript-vx-shape branch October 10, 2019 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Re-write @vx/shape in TypeScript
3 participants